fix(storage): tie-break GSI pagination on the full base primary key - #239

What

Fixes the tie-breaker used to resume a paginated Query on a global secondary index. Both backends built the ExclusiveStartKey predicate as

idx_sk > :sk OR (idx_sk = :sk AND base_sk > :base_sk)

treating the base table's sort key as the uniqueness tie-breaker. It is not one. A GSI entry is unique on (index SK, base PK, base SK): many base partitions can project the same index sort key, and those rows can also share a base sort key. When they did, both disjuncts were false and page two came back empty. The predicate is now

idx_sk > :sk OR (idx_sk = :sk AND (base_pk > :base_pk
                 OR (base_pk = :base_pk AND base_sk > :base_sk)))

and the ORDER BY gained base_pk so the ordering and the predicate agree. They have to agree: ordering by base sort key alone leaves rows that share an index sort key and a base sort key in an arbitrary order, which no ExclusiveStartKey can resume from deterministically.

Changed in crates/storage-sqlite/src/data/query_scan.rs and crates/storage-postgres/src/data/query_scan.rs. The postgres PaginationBinds doc comments in data/query.rs are corrected, because BaseSkOnly is now LSI-only and its previous comment described the broken case as the intended one.

LSI behaviour is deliberately unchanged. An LSI query always constrains the partition key, so every row shares it and the base sort key alone does identify a row. It is also a user-visible sort dimension there, so it continues to follow ScanIndexForward. Only the GSI path changed.

This also removes a divergence inside the codebase: the index scan path already ordered by the full base key (ORDER BY pk, idx_sk, base_pk, base_sk), while the query path did not.

Why

A Query on a GSI with duplicated index sort keys returned page one plus a LastEvaluatedKey, and resuming from that key returned zero items with no error. Nothing failed loudly, so a paginating client read page one and concluded it had
everything. Silent partial results are worse than an error, because the caller has no signal to retry on.

This is why a hash-only base table was unaffected and why TestGSIOnHashOnlyBaseTable passed throughout: there the base partition key is the whole base primary key, so the existing tie-breaker was already complete. Adding a range key to the base table is what exposed it.

Closes #238

Testing done

Three new tests in TestGSIOnCompositeBaseTable (tests/test_query_scan.py), against a live server on both backends, with a negative control in each case.

The test that passes in both directions is test_paginate_duplicate_gsi_sort_keys_distinct_base_sort_keys, the case the old tie-breaker could already handle. It is included so a fix that only works when the base sort key happens to break the tie cannot pass. Reverse pagination is covered too, because the base key tie-breaker stays ascending while
the index sort key reverses, so the predicate and the ORDER BY have to agree on that asymmetry.

Regression coverage:

• cargo test --workspace: 668 passed, 0 failed, 0 filtered out.
• cargo fmt --all -- --check: clean, zero diffs.
• cargo clippy --all-targets -- -D warnings: clean.
• cargo clippy --workspace --all-targets -- -W clippy::pedantic: 421 warnings,
  identical to the count on main, so this change adds none. It does grow one
  pre-existing too many lines violation in the postgres query builder from
  208/100 to 228/100; that function was already twice over the limit and
  splitting it is out of scope for a bug fix.
• Full tests/test_query_scan.py on postgres with the change: 83 passed,
  2 failed. Both failures are TestBaseKeySchemaFlow::test_index_pagination_uses_base_key_schema_for_tiebreaker
  and ::test_index_scan_pagination_uses_base_key_schema. They fail identically
  on unmodified main in my environment and pass in CI on the same commit
  (d6afa1e), so they are unrelated to this change. They are not the GSI
  propagation race either: they still fail with gsi_propagation_delay_ms set to
  0, and unlike their siblings they do not poll with wait_for_gsi_items. I have
  not root-caused that divergence and am reporting it separately rather than
  bundling it here.

Checklist

• I have read CONTRIBUTING.md
• All tests pass (cargo test --workspace) - 668 passed, 0 filtered out
• Code is formatted (cargo fmt --check)
• Clippy is clean (cargo clippy -- -W clippy::pedantic) - no new warnings
  versus main (421 both sides); see the note above on the pre-existing
  function-length violation
• I have added or updated tests for new functionality
• I have updated documentation if behavior changed - the PaginationBinds
  doc comments, which previously described the broken tie-breaker as correct
• Breaking changes are noted below (if any)
• If this changes the wire protocol, Storage trait, auth model, on-disk
  format, or public CLI surface, an RFC has been accepted or is linked
  below. Otherwise, an ADR captures the decision (link below).

ADR / RFC: n/a. No trait, wire, on-disk or CLI surface changes; this corrects
SQL generation inside two backends to match documented DynamoDB pagination
behaviour.

Breaking changes

None. LastEvaluatedKey keeps the same shape and contents, so keys issued by an
older build remain usable.

One behaviour change worth stating explicitly, though it is not breaking: the
order in which rows with tied index sort keys are returned was previously
arbitrary and is now deterministic, ordered by the base primary key. Callers
could not have depended on the old order, since it was the absence of a total
order that made pagination lose rows.

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.